Change build order #873

maccelf · 2024-06-18T12:42:55Z

github-actions · 2024-06-18T12:45:03Z

Code Coverage Summary

Package	Line Rate
alws	76%
alws.auth	77%
alws.auth.oauth	100%
alws.crud	41%
alws.dramatiq	60%
alws.middlewares	93%
alws.perms	86%
alws.routers	57%
alws.schemas	80%
alws.utils	42%
Summary	57% (5690 / 10028)

Linter reports

Pylint report

************* Module alws.build_planner
alws/build_planner.py:663:9: W0511: TODO: Make sources build as first "arch" in all process (fixme)
alws/build_planner.py:32:0: C0115: Missing class docstring (missing-class-docstring)
alws/build_planner.py:285:8: R1705: Unnecessary "elif" after "return", remove the leading "el" from "elif" (no-else-return)
alws/build_planner.py:331:4: R0914: Too many local variables (17/15) (too-many-locals)
alws/build_planner.py:382:4: C0116: Missing function or method docstring (missing-function-docstring)
alws/build_planner.py:500:4: R0914: Too many local variables (23/15) (too-many-locals)
alws/build_planner.py:500:4: R0912: Too many branches (18/12) (too-many-branches)
alws/build_planner.py:500:4: R0915: Too many statements (54/50) (too-many-statements)
************* Module alws.crud.build_node
alws/crud/build_node.py:248:0: C0301: Line too long (81/80) (line-too-long)
alws/crud/build_node.py:375:0: C0301: Line too long (82/80) (line-too-long)
alws/crud/build_node.py:431:0: C0301: Line too long (83/80) (line-too-long)
alws/crud/build_node.py:484:0: C0301: Line too long (82/80) (line-too-long)
alws/crud/build_node.py:627:0: C0301: Line too long (88/80) (line-too-long)
alws/crud/build_node.py:634:0: C0301: Line too long (81/80) (line-too-long)
alws/crud/build_node.py:786:0: C0301: Line too long (81/80) (line-too-long)
alws/crud/build_node.py:804:0: C0301: Line too long (83/80) (line-too-long)
alws/crud/build_node.py:822:0: C0301: Line too long (83/80) (line-too-long)
alws/crud/build_node.py:980:0: C0301: Line too long (85/80) (line-too-long)
alws/crud/build_node.py:995:0: C0301: Line too long (82/80) (line-too-long)
alws/crud/build_node.py:1016:0: C0301: Line too long (81/80) (line-too-long)
alws/crud/build_node.py:1018:0: C0301: Line too long (88/80) (line-too-long)
alws/crud/build_node.py:1134:0: C0301: Line too long (86/80) (line-too-long)
alws/crud/build_node.py:1:0: C0302: Too many lines in module (1148/1000) (too-many-lines)
alws/crud/build_node.py:48:5: W0511: TODO: here should be config value (fixme)
alws/crud/build_node.py:534:1: W0511: TODO: Improve readability (fixme)
alws/crud/build_node.py:688:9: W0511: TODO: Beholder doesn't have authorization right now (fixme)
alws/crud/build_node.py:1134:5: W0511: TODO: ALBS-705: Temporary solution that fixes integrity error with missing srpm, (fixme)
alws/crud/build_node.py:96:4: W0613: Unused argument 'db' (unused-argument)
alws/crud/build_node.py:124:4: C0206: Consider iterating with .items() (consider-using-dict-items)
alws/crud/build_node.py:133:0: C0116: Missing function or method docstring (missing-function-docstring)
alws/crud/build_node.py:133:0: R0914: Too many local variables (16/15) (too-many-locals)
alws/crud/build_node.py:320:0: R0914: Too many local variables (43/15) (too-many-locals)
alws/crud/build_node.py:376:12: W0707: Consider explicitly re-raising using 'raise ArtifactConversionError(f'Cannot put RPM packages into Pulp storage: {e}') from e' (raise-missing-from)
alws/crud/build_node.py:388:12: W0707: Consider explicitly re-raising using 'except Exception as exc' and 'raise RepositoryAddError(f'Cannot add RPM packages to the repository {str(repo)}') from exc' (raise-missing-from)
alws/crud/build_node.py:466:15: W0718: Catching too general exception Exception (broad-exception-caught)
alws/crud/build_node.py:486:12: W0715: Exception arguments suggest string formatting might be intended (raising-format-tuple)
alws/crud/build_node.py:320:0: R0912: Too many branches (29/12) (too-many-branches)
alws/crud/build_node.py:320:0: R0915: Too many statements (84/50) (too-many-statements)
alws/crud/build_node.py:506:8: W0707: Consider explicitly re-raising using 'raise ArtifactConversionError(f'Cannot create log files for {str(repository)}, error: {str(e)}') from e' (raise-missing-from)
alws/crud/build_node.py:528:8: W0707: Consider explicitly re-raising using 'raise RepositoryAddError('Cannot save build log into Pulp repository: %s', str(e)) from e' (raise-missing-from)
alws/crud/build_node.py:528:8: W0715: Exception arguments suggest string formatting might be intended (raising-format-tuple)
alws/crud/build_node.py:537:0: R0914: Too many local variables (39/15) (too-many-locals)
alws/crud/build_node.py:721:17: W1309: Using an f-string that does not have any interpolated variables (f-string-without-interpolation)
alws/crud/build_node.py:722:17: W1309: Using an f-string that does not have any interpolated variables (f-string-without-interpolation)
alws/crud/build_node.py:785:24: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
alws/crud/build_node.py:537:0: R0912: Too many branches (15/12) (too-many-branches)
alws/crud/build_node.py:537:0: R0915: Too many statements (102/50) (too-many-statements)
alws/crud/build_node.py:899:19: C0209: Formatting a regular string which could be an f-string (consider-using-f-string)
alws/crud/build_node.py:975:0: C0116: Missing function or method docstring (missing-function-docstring)
alws/crud/build_node.py:991:11: W0718: Catching too general exception Exception (broad-exception-caught)
alws/crud/build_node.py:1043:0: C0116: Missing function or method docstring (missing-function-docstring)
alws/crud/build_node.py:1043:0: R0914: Too many local variables (16/15) (too-many-locals)
alws/crud/build_node.py:17:0: W0611: Unused GitHubIssueStatus imported from alws.constants (unused-import)
************* Module alws.crud.test
alws/crud/test.py:66:0: C0116: Missing function or method docstring (missing-function-docstring)
alws/crud/test.py:211:0: C0116: Missing function or method docstring (missing-function-docstring)
alws/crud/test.py:337:0: C0116: Missing function or method docstring (missing-function-docstring)
alws/crud/test.py:385:8: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
alws/crud/test.py:388:8: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
alws/crud/test.py:423:0: C0116: Missing function or method docstring (missing-function-docstring)
alws/crud/test.py:423:0: R0914: Too many local variables (17/15) (too-many-locals)
alws/crud/test.py:477:11: W0718: Catching too general exception Exception (broad-exception-caught)
alws/crud/test.py:461:4: R1702: Too many nested blocks (6/5) (too-many-nested-blocks)
alws/crud/test.py:423:0: R0912: Too many branches (17/12) (too-many-branches)
************* Module alws.routers.build_node
alws/routers/build_node.py:132:5: W0511: TODO: Get rid of this fixes when all affected builds would be processed (fixme)
alws/routers/build_node.py:59:0: C0116: Missing function or method docstring (missing-function-docstring)
alws/routers/build_node.py:70:25: C0209: Formatting a regular string which could be an f-string (consider-using-f-string)
alws/routers/build_node.py:59:0: R0912: Too many branches (24/12) (too-many-branches)

-----------------------------------
Your code has been rated at 9.40/10

Black report

--- alws/build_planner.py	2024-06-21 14:32:56.713277+00:00
+++ alws/build_planner.py	2024-06-21 14:35:11.850848+00:00
@@ -68,13 +68,11 @@
             if 'i686' in arch_list and arch_list.index('i686') != 0:
                 arch_list.remove('i686')
                 arch_list.insert(0, 'i686')
             self._request_platforms_arch_list[platform.name] = arch_list
 
-            self._parallel_modes[platform.name] = (
-                platform.parallel_mode_enabled
-            )
+            self._parallel_modes[platform.name] = platform.parallel_mode_enabled
         await self.load_platforms()
         if platform_flavors:
             await self.load_platform_flavors(platform_flavors)
         self._gitea_client = GiteaClient(
             settings.gitea_host, logging.getLogger(__name__)
@@ -195,13 +193,11 @@
                 if arch == 'src':
                     continue
 
                 tasks.append(self.create_build_repo(platform, arch, 'rpm'))
                 tasks.append(
-                    self.create_build_repo(
-                        platform, arch, 'rpm', is_debug=True
-                    )
+                    self.create_build_repo(platform, arch, 'rpm', is_debug=True)
                 )
 
             # Add source RPM repository
             tasks.append(self.create_build_repo(platform, 'src', 'rpm'))
 
@@ -533,12 +529,11 @@
                     modularity_version = flavour.modularity['versions'][-1]
             if task.module_platform_version:
                 flavour_versions = [
                     flavour.modularity['versions']
                     for flavour in self._platform_flavors
-                    if flavour.modularity
-                    and flavour.modularity.get('versions')
+                    if flavour.modularity and flavour.modularity.get('versions')
                 ]
                 modularity_version = next(
                     item
                     for item in itertools.chain(
                         platform.modularity['versions'], *flavour_versions
--- alws/crud/build_node.py	2024-06-21 14:32:56.713277+00:00
+++ alws/crud/build_node.py	2024-06-21 14:35:12.081720+00:00
@@ -243,11 +243,13 @@
     pulp_repo = await pulp_client.get_log_repository(repo_name)
     if pulp_repo:
         pulp_href = pulp_repo["pulp_href"]
         distro = await pulp_client.get_log_distro(repo_name)
         if not distro:
-            repo_url = await pulp_client.create_file_distro(repo_name, pulp_href)
+            repo_url = await pulp_client.create_file_distro(
+                repo_name, pulp_href
+            )
         else:
             repo_url = distro["base_url"]
     else:
         repo_url, pulp_href = await pulp_client.create_log_repo(repo_name)
     if await log_repo_exists(db, task):
@@ -370,11 +372,13 @@
         if not tasks:
             continue
         try:
             results = await asyncio.gather(*tasks)
         except Exception as e:
-            logging.exception("Cannot create RPM packages for repo %s", str(repo))
+            logging.exception(
+                "Cannot create RPM packages for repo %s", str(repo)
+            )
             raise ArtifactConversionError(
                 f"Cannot put RPM packages into Pulp storage: {e}"
             )
         processed_packages.extend(results)
         hrefs = [item[0] for item in results]
@@ -426,11 +430,13 @@
             models.NewErrataPackage.version == rpm_info["version"],
         ]
         if rpm_info["arch"] != "noarch":
             conditions.append(models.NewErrataPackage.arch == rpm_info["arch"])
 
-        query = select(models.NewErrataPackage).where(sqlalchemy.and_(*conditions))
+        query = select(models.NewErrataPackage).where(
+            sqlalchemy.and_(*conditions)
+        )
 
         if module_index:
             module = None
             for mod in module_index.iter_modules():
                 if mod.name.endswith("-devel"):
@@ -479,11 +485,13 @@
             module_artifacts.append(srpms_info[db_srpm.href])
     if module_index and module_artifacts:
         try:
             for module in module_index.iter_modules():
                 for artifact in module_artifacts:
-                    module.add_rpm_artifact(artifact, task_excluded=task_excluded)
+                    module.add_rpm_artifact(
+                        artifact, task_excluded=task_excluded
+                    )
         except Exception as e:
             raise ModuleUpdateError("Cannot update module: %s", str(e)) from e
 
     return rpms
 
@@ -622,18 +630,22 @@
             build_repo
             for build_repo in rpm_repositories
             if build_repo.arch == build_task.arch and build_repo.debug is False
         )
         try:
-            repo_modules_yaml = await pulp_client.get_repo_modules_yaml(module_repo.url)
+            repo_modules_yaml = await pulp_client.get_repo_modules_yaml(
+                module_repo.url
+            )
             module_index = IndexWrapper.from_template(repo_modules_yaml)
         except Exception as e:
             message = f"Cannot parse modules index: {str(e)}"
             logging.exception("Cannot parse modules index: %s", str(e))
             raise ModuleUpdateError(message) from e
     rpm_artifacts = [item for item in task_artifacts if item.type == "rpm"]
-    log_artifacts = [item for item in task_artifacts if item.type == "build_log"]
+    log_artifacts = [
+        item for item in task_artifacts if item.type == "build_log"
+    ]
     src_rpm = _get_srpm_name(
         artifacts=task_artifacts,
         task=build_task,
     )
     # If we have RPM artifacts but missing source RPM
@@ -799,11 +811,13 @@
                     rpm_module.context,
                     rpm_module.arch,
                     module_for_pulp.description,
                     version=module_version,
                     artifacts=module_for_pulp.get_rpm_artifacts(),
-                    dependencies=list(module_for_pulp.get_runtime_deps().values()),
+                    dependencies=list(
+                        module_for_pulp.get_runtime_deps().values()
+                    ),
                     # packages=module_pkgs_hrefs,
                     packages=[],
                     profiles=module_for_pulp.get_profiles(),
                 )
                 # Here we ensure that we add the recently created module
@@ -817,11 +831,13 @@
                     "start_ts": str(start_time),
                     "end_ts": str(end_time),
                     "delta": str(end_time - start_time),
                 }
             except Exception as e:
-                message = f"Cannot update module information inside Pulp: {str(e)}"
+                message = (
+                    f"Cannot update module information inside Pulp: {str(e)}"
+                )
                 logging.exception(message)
                 raise ModuleUpdateError(message) from e
             logging.info("Module template processing is finished")
         await pulp_client.modify_repository(
             module_repo.pulp_href,
@@ -975,11 +991,13 @@
 async def safe_build_done(
     db: AsyncSession,
     request: build_node_schema.BuildDone,
 ):
     success = True
-    pulp = PulpClient(settings.pulp_host, settings.pulp_user, settings.pulp_password)
+    pulp = PulpClient(
+        settings.pulp_host, settings.pulp_user, settings.pulp_password
+    )
     build_task_stats = {
         "build_node_stats": request.stats,
         "build_done_stats": {},
     }
     start_time = datetime.datetime.utcnow()
@@ -990,11 +1008,13 @@
             await db.flush()
     except Exception:
         logging.exception("Build done failed:")
         success = False
         build_task = await db.execute(
-            select(models.BuildTask).where(models.BuildTask.id == request.task_id)
+            select(models.BuildTask).where(
+                models.BuildTask.id == request.task_id
+            )
         )
         build_task = build_task.scalars().first()
         build_task.ts = datetime.datetime.utcnow()
         build_task.status = BuildTaskStatus.FAILED
         build_task.error = traceback.format_exc()
@@ -1011,13 +1031,16 @@
         }
         await __update_built_srpm_url(db, build_task, request)
         await db.flush()
     finally:
         remove_dep_query = delete(models.BuildTaskDependency).where(
-            models.BuildTaskDependency.c.build_task_dependency == request.task_id
-        )
-        build_task_start_time = request.stats.get("build_node_task", {}).get("start_ts")
+            models.BuildTaskDependency.c.build_task_dependency
+            == request.task_id
+        )
+        build_task_start_time = request.stats.get("build_node_task", {}).get(
+            "start_ts"
+        )
         if build_task_start_time:
             build_task_start_time = datetime.datetime.fromisoformat(
                 build_task_start_time
             )
         await db.execute(

Bandit report

Run started:2024-06-21 14:35:14.040988

Test results:
	No issues identified.

Code scanned:
	Total lines of code: 2303
	Total lines skipped (#nosec): 0
	Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
	Total issues (by severity):
		Undefined: 0
		Low: 0
		Medium: 0
		High: 0
	Total issues (by confidence):
		Undefined: 0
		Low: 0
		Medium: 0
		High: 0
Files skipped (0):

View full reports on the Job Summary page.

Korulag · 2024-06-18T12:49:54Z

alws/build_planner.py

@@ -660,9 +666,16 @@ async def build_dependency_map(self):
        #   - All architectures should have dependencies
        #     between their own tasks to ensure correct build order.
        all_tasks = []
+        priority_arches = ['src', 'i686']


All arch tasks should be dependent only on src task, so first arch will always be src, even when the list of arches is custom

I added i686 as the second priority arch to leave the possibility of building from src. For this case 'i686' will be a priority arch as it is now

Are you sure that if I leave only src then I didn't break a multilib?

Multilib is triggered when x86_64 arch is finishing, and at planner time it's only a question of modules. BTW, please check modules workflow: does it remain the same as now?

Checked modules. Now all works fine

Korulag · 2024-06-18T12:52:05Z

alws/routers/build_node.py

+        supported_arches.remove('src')
+    task_arch = task.arch
+    if task_arch == 'src':
+        task_arch = supported_arches[0]


What if src will not be the first arch?

We don't need src as the first arch in supported_arches that's why I removed it

Okay, but what will happen when you finishing src task? It looks like this way you'll be treating src task like e.g. aarch64 task which is not fully correct

No that is exactly what I am trying to do here. This part of the code adds repositories for a builder to build RPM. So because we can build src on any builders we need to add here only repositories for builder supported arch.

Resolves: AlmaLinux/build-system#212

Korulag reviewed Jun 18, 2024

View reviewed changes

maccelf marked this pull request as draft June 19, 2024 13:14

Change build order

9f973ee

Resolves: AlmaLinux/build-system#212

maccelf force-pushed the add_src_arch branch from 964c220 to 9f973ee Compare June 21, 2024 14:32

maccelf marked this pull request as ready for review June 21, 2024 14:40

javihernandez approved these changes Jun 28, 2024

View reviewed changes

soksanichenko approved these changes Jul 1, 2024

View reviewed changes

maccelf merged commit 73ddda2 into master Jul 2, 2024
2 checks passed

maccelf deleted the add_src_arch branch July 2, 2024 11:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change build order #873

Change build order #873

maccelf commented Jun 18, 2024

github-actions bot commented Jun 18, 2024 •

edited

Loading

Korulag Jun 18, 2024

maccelf Jun 18, 2024

maccelf Jun 18, 2024

Korulag Jun 18, 2024

maccelf Jun 21, 2024

Korulag Jun 18, 2024

maccelf Jun 18, 2024

Korulag Jun 18, 2024

maccelf Jun 19, 2024

Change build order #873

Change build order #873

Conversation

maccelf commented Jun 18, 2024

github-actions bot commented Jun 18, 2024 • edited Loading

Code Coverage Summary

Linter reports

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 18, 2024 •

edited

Loading